Skip to content
This repository was archived by the owner on Mar 21, 2024. It is now read-only.

Avoid "statement is unreachable" compile errors in async_reduce.cu test#1232

Merged
alliepiper merged 1 commit intomainfrom
bug/test/async_reduce
Apr 7, 2021
Merged

Avoid "statement is unreachable" compile errors in async_reduce.cu test#1232
alliepiper merged 1 commit intomainfrom
bug/test/async_reduce

Conversation

@dkolsen-pgi
Copy link
Copy Markdown
Collaborator

Test test_async_reduce_allocator_on_then_after in async_reduce.cu has
"KNOWN_FAILURE;" in the middle of a code block. This results in "statement is
unreachable" compiler errors from NVC++ on the following line.

"/proj/cuda/thrust/main/testing/async_reduce.cu", line 978: error: statement is
          unreachable
      ASSERT_EQUAL_QUIET(stream1, f2.stream().native_handle());
      ^
          detected during:
            instantiation of "void unittest::for_each_type<TypeList, Function,
                      T, i>::operator()(U) [with TypeList=NumericTypes,
                      Function=test_async_reduce_allocator_on_then_after,
                      T=char, i=0U, U=size_t]" at line 537 of
                      "/proj/cuda/thrust/main/testing/unittest/testframework.h"
            instantiation of "void VariableUnitTest<TestName, TypeList>::run()
                      [with TestName=test_async_reduce_allocator_on_then_after,
                      TypeList=NumericTypes]"

The compiler error is correct. KNOWN_FAILURE expands to a throw expression,
so everything after it is dead code.

Fix the problem by putting everything after KNOWN_FAILURE in a "#if 0" block.

Test test_async_reduce_allocator_on_then_after in async_reduce.cu has
"KNOWN_FAILURE;" in the middle of a code block.  This results in "statement is
unreachable" compiler errors from NVC++ on the following line.

```
"/proj/cuda/thrust/main/testing/async_reduce.cu", line 978: error: statement is
          unreachable
      ASSERT_EQUAL_QUIET(stream1, f2.stream().native_handle());
      ^
          detected during:
            instantiation of "void unittest::for_each_type<TypeList, Function,
                      T, i>::operator()(U) [with TypeList=NumericTypes,
                      Function=test_async_reduce_allocator_on_then_after,
                      T=char, i=0U, U=size_t]" at line 537 of
                      "/proj/cuda/thrust/main/testing/unittest/testframework.h"
            instantiation of "void VariableUnitTest<TestName, TypeList>::run()
                      [with TestName=test_async_reduce_allocator_on_then_after,
                      TypeList=NumericTypes]"
```

The compiler error is correct.  KNOWN_FAILURE expands to a throw expression,
so everything after it is dead code.

Fix the problem by putting everything after KNOWN_FAILURE in a "#if 0" block.
@alliepiper alliepiper added this to the 1.9.10-1 milestone Jul 15, 2020
@alliepiper
Copy link
Copy Markdown
Collaborator

CL 28814105

@alliepiper
Copy link
Copy Markdown
Collaborator

CI looks good. @brycelelbach can you merge this along with the other 1.9.10-1 integrations you're doing today?

@alliepiper alliepiper added the testing: internal ci passed Passed internal NVIDIA CI (DVS). label Jul 16, 2020
@brycelelbach
Copy link
Copy Markdown
Collaborator

I don't think this is the right fix; just move KNOWN_FAILURE to the end of the file instead. Also, this should be fixed by an upcoming PR to not be a known failure.

@brycelelbach brycelelbach modified the milestones: 1.9.10-1, 1.10.0 Jul 16, 2020
@dkolsen-pgi
Copy link
Copy Markdown
Collaborator Author

If KNOWN_FAILURE were moved to the end of the function, then (if I am interpreting the comments in the code correctly), at least one of the tests that follow will fail before the KNOWN_FAILURE is hit. I assume that will mark the test as failed in the test results. I think the proposed fix is the correct one. I don't care whether or not this fix gets into 1.9.10-1; its known to be a test bug, not a compiler bug, so the failure won't hold up anything.

@alliepiper
Copy link
Copy Markdown
Collaborator

@brycelelbach Are you still opposed to this approach, or should I start CI on this PR?

@brycelelbach brycelelbach self-assigned this Aug 5, 2020
@brycelelbach brycelelbach added the compiler: nvc++ Specific to the NVC++ compiler. label Aug 31, 2020
@brycelelbach
Copy link
Copy Markdown
Collaborator

No this will be fixed by #1195

@brycelelbach brycelelbach deleted the bug/test/async_reduce branch September 26, 2020 00:39
@dkolsen-pgi dkolsen-pgi restored the bug/test/async_reduce branch March 9, 2021 00:37
@brycelelbach brycelelbach reopened this Mar 9, 2021
@GPUtester
Copy link
Copy Markdown
Collaborator

Can one of the admins verify this patch?

@brycelelbach brycelelbach modified the milestones: 1.10.0, 1.13.0 Mar 9, 2021
@brycelelbach
Copy link
Copy Markdown
Collaborator

We decided to just go ahead and merge this workaround now as the timeline for me landing the proper fix is unclear.

@brycelelbach brycelelbach added P1: should have Necessary, but not critical. nvc++ flyspray Has an associated internal NVIDIA NVC++ flyspray. labels Mar 29, 2021
@alliepiper alliepiper merged commit b7be11f into main Apr 7, 2021
@alliepiper alliepiper deleted the bug/test/async_reduce branch April 7, 2021 16:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

compiler: nvc++ Specific to the NVC++ compiler. nvc++ flyspray Has an associated internal NVIDIA NVC++ flyspray. P1: should have Necessary, but not critical. testing: internal ci passed Passed internal NVIDIA CI (DVS).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants